Remove Giving Nodes a Type #4040
Conversation
This is deprecated in favor of the new Type System.
MichelDiz
requested review from
danielmai and
MichaelJCompton
as code owners
![pullrequest[bot]](https://avatars.githubusercontent.com/in/3434?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ A review job has been created and sent to the PullRequest network.
@MichelDiz you can click here to see the review status or cancel the code review job.
There was a problem hiding this comment.
Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @danielmai, @MichaelJCompton, and @MichelDiz)
wiki/content/howto/index.md, line 377 at r1 (raw file):
Quoted 12 lines of code…
It's often useful to give the nodes in a graph *types* (also commonly referred to as *labels* or *kinds*). This allows you to do lots of useful things. For example: - Search for all nodes of a certain type in the root function. - Filter nodes to only be of a certain kind. - Enable easier exploration and understanding of a dataset. Graphs are easier to grok when there's an explicit type for each node, since there's a clearer expectation about what predicates it may or may not have.
Really like the idea of removing the section. My only question is if this kind of description now belongs at the top of the types section in the docs? Or do you think that by just calling it types removes the need for any of this?
There was a problem hiding this comment.
Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @danielmai and @MichaelJCompton)
wiki/content/howto/index.md, line 377 at r1 (raw file):
Previously, MichaelJCompton (Michael Compton) wrote…
It's often useful to give the nodes in a graph *types* (also commonly referred to as *labels* or *kinds*). This allows you to do lots of useful things. For example: - Search for all nodes of a certain type in the root function. - Filter nodes to only be of a certain kind. - Enable easier exploration and understanding of a dataset. Graphs are easier to grok when there's an explicit type for each node, since there's a clearer expectation about what predicates it may or may not have.Really like the idea of removing the section. My only question is if this kind of description now belongs at the top of the types section in the docs? Or do you think that by just calling it types removes the need for any of this?
This session came with time becoming unnecessary, I believe it was because of it that we started the idea of having a Type System. I have already highly recommended this approach from that session myself. But the use of "has ()" is somewhat counter-prudent at root. (Although we are always improving has()func over time).
As for the description, I cannot say how we could convey its meaning there. But it is a fact that this approach is inefficient than using the Type System. (but will not be removed, only discouraged and removed from docs*).
There was a problem hiding this comment.
Due to inactivity, PullRequest has cancelled this review job. You can reactivate the code review job from the PullRequest dashboard
There was a problem hiding this comment.
Reviewed 1 of 1 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @danielmai and @MichaelJCompton)
There was a problem hiding this comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @danielmai and @MichaelJCompton)
There was a problem hiding this comment.
Not sure, I think I could seek for pointing URLs and eliminate them. What I'm not sure is about left deprecated info in the docs and just left a "note" about it. If this is a good move in terms of maintaining good documentation.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @MichaelJCompton)
There was a problem hiding this comment.
Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @martinmr and @MichaelJCompton)
* Remove Giving Nodes a Type This is deprecated in favor of the new Type System. * add Dmai's suggestion of leaving a reference there. (cherry picked from commit 8a5c2f2)
This is deprecated in favor of the new Type System.
This change is